gossip2, snaprd: plumbing for gossip2 to snaprd tile contact info updates #6003
Conversation
src/discof/restore/fd_snaprd_tile.c
Outdated
| switch( msg->tag ) { | ||
| case FD_GOSSIP_UPDATE_TAG_CONTACT_INFO: { | ||
| fd_contact_info_upd_t * cur = &ctx->gossip.ci_table[ msg->contact_info.idx ]; | ||
| fd_ip4_port_t cur_addr = ctx->gossip.ci_table[ msg->contact_info.idx ].sockets[ FD_CONTACT_INFO_SOCKET_RPC ]; |
| first, to ensure there's only one control message outstanding | ||
| at a time. */ | ||
| ctx->malformed = 1; | ||
| if( FD_UNLIKELY( ctx->in_kind[ in_idx ]==IN_KIND_GOSSIP ) ) { |
There was a problem hiding this comment.
I kinda think you can get away with matching the in_idx with a predefined constant.
There was a problem hiding this comment.
Predefined based on ordering in topology.c? Isn't that a little fragile? Also the actual link set up in topology.c will come with the full gossip PR, so until then we don't have a constant (I guess a placeholder would work)
There was a problem hiding this comment.
yeah... but the other tiles do it too? idk though up to you.
src/discof/restore/fd_snaprd_tile.c
Outdated
| } | ||
| fd_contact_info_upd_t * new = msg->contact_info.contact_info; | ||
| fd_ip4_port_t new_addr = new->sockets[ FD_CONTACT_INFO_SOCKET_RPC ]; | ||
| if( new_addr.l ) { |
There was a problem hiding this comment.
Also think we need to check if new_addr is equal to cur_addr so that we don't unnecessarily remove and add the same contact info.
This is what I have:
if( cur_addr.l!=new_addr.l ) {
fd_ssping_remove( ctx->ssping, cur_addr );
if( new_addr.l ) {
FD_LOG_INFO(("adding contact info for peer "FD_IP4_ADDR_FMT ":%hu ",
FD_IP4_ADDR_FMT_ARGS( new_addr.addr ), fd_ushort_bswap( new_addr.port ) ));
fd_ssping_add( ctx->ssping, new_addr );
}
*cur = *new;
}
src/discof/restore/fd_snaprd_tile.c
Outdated
| @@ -98,6 +112,11 @@ struct fd_snaprd_tile { | |||
| } incremental; | |||
| } metrics; | |||
|
|
|||
| struct { | |||
| fd_gossip_update_message_t tmp_upd_buf; | |||
| fd_contact_info_upd_t * ci_table; | |||
There was a problem hiding this comment.
Don't think this needs to be the full update? Just the IP address, right?
src/discof/restore/fd_snaprd_tile.c
Outdated
| case FD_GOSSIP_UPDATE_TAG_CONTACT_INFO: { | ||
| fd_contact_info_upd_t * cur = &ctx->gossip.ci_table[ msg->contact_info.idx ]; | ||
| fd_ip4_port_t cur_addr = ctx->gossip.ci_table[ msg->contact_info.idx ].sockets[ FD_CONTACT_INFO_SOCKET_RPC ]; | ||
| if( cur_addr.l ){ |
There was a problem hiding this comment.
| if( cur_addr.l ){ | |
| if( FD_LIKELY( cur_addr.l ) ) fd_ssping_remove( ctx->ssping, cur_addr ); |
src/discof/restore/fd_snaprd_tile.c
Outdated
| fd_gossip_update_message_t * msg = &ctx->gossip.tmp_upd_buf; | ||
| switch( msg->tag ) { | ||
| case FD_GOSSIP_UPDATE_TAG_CONTACT_INFO: { | ||
| fd_contact_info_upd_t * cur = &ctx->gossip.ci_table[ msg->contact_info.idx ]; |
There was a problem hiding this comment.
| fd_contact_info_upd_t * cur = &ctx->gossip.ci_table[ msg->contact_info.idx ]; |
| for( ulong i=0UL; i<1UL; i++ ) fd_ssping_add( ctx->ssping, initial_peers[ i ] ); | ||
| } else if (FD_LIKELY( !strcmp( tile->snaprd.cluster, "mainnet" ) ) ) { | ||
| ctx->gossip.ci_table = _ci_table; | ||
| /* zero-out memory so that we can perform null checks in after_frag */ |
There was a problem hiding this comment.
I don't think you need to do this?
There was a problem hiding this comment.
This would affect the if( FD_LIKELY( !!cur_addr.l ) ) checks in after_frag for a new entry
There was a problem hiding this comment.
You should have issued a REMOVE first, so that line is pointless and should never be needed, right? It actually seems wrong because you might release an IP that wasn't in use from gossip
There was a problem hiding this comment.
You should have issued a REMOVE first
CONTACT_INFO_REMOVE is only issued if a contact info was dropped from the Gossip table, not if only its RPC socket entry is removed. The removal of a valid contact info's RPC socket is communicated via the normal CONTACT_INFO update message. So we'd need null checks to track transitions (no addr > some addr, some addr > no addr, old addr > new addr)
| ulong ctl FD_PARAM_UNUSED) { | ||
| if( ctx->in_kind[ in_idx ]!= IN_KIND_GOSSIP ) return; | ||
|
|
||
| if( FD_UNLIKELY( chunk<ctx->gossip_in.chunk0 || |
There was a problem hiding this comment.
Check sz against mtu as well, see other tile examples
ravyu-jump
force-pushed
the
gossip2-snaprd-plumbing
branch
3 times, most recently
from
665e488 to
4d7286a
Compare
| if( FD_UNLIKELY( chunk<ctx->gossip_in.chunk0 || | ||
| chunk>ctx->gossip_in.wmark || | ||
| sz>sizeof(fd_gossip_update_message_t) ) ) { | ||
| FD_LOG_ERR(( "snaprd: unexpected chunk %lu", chunk )); |
There was a problem hiding this comment.
Could use FD_TEST here for simplicity.
ravyu-jump
force-pushed
the
gossip2-snaprd-plumbing
branch
from
4d7286a to
f361a55
Compare
No description provided.